You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is a wacky change I've wanted to try out for a while and finally started screwing around with with copilot.
Right now (and in Strada), we have just two kinds of paths:
string - 🤷
Path - an OS dependent string used for map keys, lowercased on case insensitive systems
Our use of string paths led to us slapping normalizeSlashes, normalizePath, etc everywhere, as we often were unsure (or pessimistic) whether or not a path had its slashes normalized to /, had redundant components removed, trailing slashes removed, not relative, etc. This is extra bad because on Linux, macOS, etc, paths are basically guaranteed to meet all of the criteria, but we'd try and normalize them anyway.
This PR changes this by introducing named/branded types for paths which assert properties about those paths. This is not a new concept; I believe yarn's FS package has this, and I'm sure others do.
As a hierarchy:
string - No guarantees.
RootedPath - The path is absolute, has normalized slashes, no trailing /.
RootedFilePath - A RootedPath, but indicates that the path is supposed to point at a file.
RootedDirectoryPath - A RootedPath, but indicates that the path is supposed to point at a directory.
PathKey - Same as the old Path, but renamed for clarity.
This is a big refactor that requires changing a lot of code, but leads to some pretty important properties.
Paths are converted at the boundaries, e.g. paths provided via config files, CLI, from the OS, the editor, etc. Once converted, you always know exactly what format a path is in and therefore never need to normalize again.
Paths are always rooted. The "current working directory" does not need to be plumbed around as much anymore, since most uses were simply to root paths we were unsure about.
Since paths are always rooted, ComparePathsOptions's current dir field is no longer needed! This means comparing paths only requires UseCaseSensitiveFileNames. This applies also to all of our old toPath conversions, since we only ever need to canonicalize rooted paths. So, I created a new CaseSensitivity enum, and then all of the plumbing for ComparePathsOptions, its working dir, etc, also get to go away.
The impact of this is measurable; I instrumented main vs my branch to count how many of the normalizing operations go away and it's a lot:
Old compiler fixture
Metric
main
typed-paths
Change
Absolute rooting
1,228
6
-99.511%
Canonicalize
27,213
1,115
-95.903%
CombinePaths
2,145
7
-99.674%
Lowercase
189
189
unchanged
NormalizePath
27,567
2
-99.993%
NormalizeSlashes
35,693
588
-98.353%
Total path-key construction
26,689
1,115
-95.822%
VS Code src
Metric
main
typed-paths
Change
Absolute rooting
237,348
851
-99.641%
Canonicalize
878,794
199,944
-77.248%
CombinePaths
234,338
156
-99.933%
Lowercase
7,996
7,996
unchanged
NormalizePath
971,125
6
-99.999%
NormalizeSlashes
2,507,002
8,720
-99.652%
Total path-key construction
735,489
116,736
-84.128%
That's millions of normalizations that no longer need to happen. In terms of runtime, it's not a lot of savings, even on Windows, but I did also measure about a 7% speedup in program load of the old compiler, which is nice.
In the course of this PR, copilot found 15 bugs. 8 of which were unrelated but noticed as the files were being read, but 7 of which were related to typed paths. 4 of those bugs were also present in Strada!
Additionally, the strong typing here caught 3 different bugs that have been around in main for a while, places where we had mixed up paths, rooted them relative to the wrong directory, etc. Those are denoted in my (awful) git history as being things to port to main, which I may still do.
In addition to just the types themselves, a new lint rule bans manually hacking on the paths; all operations should go through methods on the paths themselves. No concat, splitting, conversions, yourself.
The downside here is just churning the API and introducing these concepts to downstream API users. But the strong typing itself I think is worth it, and doing a lot less work is a bonus too. We probably won't have a change to do something like this for a while.
I'm also going to say that this fixes#44174 just since this eliminates nearly all normalization; we might still do a quick check at the boundaries, but other than that, we never normalize gain.
realpath callbacks are allowed to return a relative path: the server resolves that value against the queried path's directory (callbackfs.go:283-299), and the sync client forwards it unchanged. Rooting it here with no current directory makes only the async client throw for a valid result such as ../real/file.ts. Forward the callback result and let the server perform the boundary conversion.
Replace ambiguous string path contracts with a typed lattice for rooted
files, rooted directories, normalized relative paths, and canonical path
keys. Keep canonical identity as a one-way sink while retaining
presentation spelling wherever diagnostics, watches, symlinks, or
protocol responses need it.
Carry those invariants through compiler inputs and outputs, module
resolution, project snapshots, language-service hosts, VFS operations,
source maps, LSP conversion, and the JavaScript API. Separate raw
compiler option wire values from finalized rooted options, and
centralize explicit normalization, rooting, and case-sensitivity
boundaries.
Keep absent and empty sourceRoot values equivalent when decoding source
maps. This preserves published map compatibility without weakening
rooted-file invariants or selecting sources by file existence.
Preserve the host casing policy when aggregating watcher directories.
Presentation spelling must not prevent case-insensitive paths from
sharing a watch, and case-sensitive paths must remain distinct.
This commit consolidates the exploratory migration into one reviewable
rewrite after the independently portable fixes. It also adapts those
fixes to the typed representation and retains the two newer main
changes, including auto-import completion retries and tuple completion
filtering.
Category: Typed-path migration
Keep path branding internal to finalized compiler state while allowing API
callers to continue supplying ordinary strings. Normalize those values at
API boundaries using generated compiler-option path metadata.
The path-type migration unintentionally changed the test filesystem
from case-insensitive to case-sensitive. Keep coverage of canonicalized
file identities and guard the test filesystem policy.
Custom diagnostic hosts should continue accepting ordinary directory
strings, including spellings that need normalization. Cover this API
boundary and remove the stale branded-directory imports.
Relative realpath callback results are resolved by the server against
the queried path's directory. Normalizing them without a base in the
async client rejects valid results and differs from the sync client.
Valid filenames such as .ts, ..ts, and ...ts lose their normalized path
structure when their extensions are removed. Treating those incomplete
prefixes as rooted paths can panic or misidentify their directory.
Keep incomplete filename prefixes separate so resolution, output
naming, and renaming preserve literal filenames without weakening
rooted-path invariants.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a wacky change I've wanted to try out for a while and finally started screwing around with with copilot.
Right now (and in Strada), we have just two kinds of paths:
string- 🤷Path- an OS dependent string used for map keys, lowercased on case insensitive systemsOur use of
stringpaths led to us slappingnormalizeSlashes,normalizePath, etc everywhere, as we often were unsure (or pessimistic) whether or not a path had its slashes normalized to/, had redundant components removed, trailing slashes removed, not relative, etc. This is extra bad because on Linux, macOS, etc, paths are basically guaranteed to meet all of the criteria, but we'd try and normalize them anyway.This PR changes this by introducing named/branded types for paths which assert properties about those paths. This is not a new concept; I believe yarn's FS package has this, and I'm sure others do.
As a hierarchy:
string- No guarantees.RootedPath- The path is absolute, has normalized slashes, no trailing/.RootedFilePath- ARootedPath, but indicates that the path is supposed to point at a file.RootedDirectoryPath- ARootedPath, but indicates that the path is supposed to point at a directory.PathKey- Same as the oldPath, but renamed for clarity.This is a big refactor that requires changing a lot of code, but leads to some pretty important properties.
Paths are converted at the boundaries, e.g. paths provided via config files, CLI, from the OS, the editor, etc. Once converted, you always know exactly what format a path is in and therefore never need to normalize again.
Paths are always rooted. The "current working directory" does not need to be plumbed around as much anymore, since most uses were simply to root paths we were unsure about.
Since paths are always rooted,
ComparePathsOptions's current dir field is no longer needed! This means comparing paths only requiresUseCaseSensitiveFileNames. This applies also to all of our oldtoPathconversions, since we only ever need to canonicalize rooted paths. So, I created a newCaseSensitivityenum, and then all of the plumbing forComparePathsOptions, its working dir, etc, also get to go away.The impact of this is measurable; I instrumented main vs my branch to count how many of the normalizing operations go away and it's a lot:
Old compiler fixture
maintyped-pathsCombinePathsNormalizePathNormalizeSlashesVS Code
srcmaintyped-pathsCombinePathsNormalizePathNormalizeSlashesThat's millions of normalizations that no longer need to happen. In terms of runtime, it's not a lot of savings, even on Windows, but I did also measure about a 7% speedup in program load of the old compiler, which is nice.
In the course of this PR, copilot found 15 bugs. 8 of which were unrelated but noticed as the files were being read, but 7 of which were related to typed paths. 4 of those bugs were also present in Strada!
Additionally, the strong typing here caught 3 different bugs that have been around in main for a while, places where we had mixed up paths, rooted them relative to the wrong directory, etc. Those are denoted in my (awful) git history as being things to port to main, which I may still do.
In addition to just the types themselves, a new lint rule bans manually hacking on the paths; all operations should go through methods on the paths themselves. No concat, splitting, conversions, yourself.
The downside here is just churning the API and introducing these concepts to downstream API users. But the strong typing itself I think is worth it, and doing a lot less work is a bonus too. We probably won't have a change to do something like this for a while.
I'm also going to say that this fixes #44174 just since this eliminates nearly all normalization; we might still do a quick check at the boundaries, but other than that, we never normalize gain.